android chromium-130 changes #86
Conversation
| @@ -372,7 +372,8 @@ public void run() { | |||
|
|
|||
| private void setNtpRecyclerView(LinearLayoutManager linearLayoutManager) { | |||
| mIsTopSitesEnabled = NtpUtil.shouldDisplayTopSites(); | |||
| mIsBraveStatsEnabled = NtpUtil.shouldDisplayBraveStats(); | |||
There was a problem hiding this comment.
return false from inside this function instead. we wouldn't have to make changes at multiple places then
| @@ -85,21 +85,21 @@ protected void onFinishInflate() { | |||
| // any shortcut causes the UrlBar to be focused. See ViewRootImpl.leaveTouchMode(). | |||
| mScrollView.setDescendantFocusability(FOCUS_BEFORE_DESCENDANTS); | |||
|
|
|||
| mVpnCta = findViewById(R.id.tv_try_vpn); | |||
| if (BraveVpnUtils.isVpnFeatureSupported(getContext()) | |||
There was a problem hiding this comment.
if we just added a false in this If block then then else block would have hidden this as well? and the number of changes would become minimum which reduces the chances of conflicts later on
| .readBoolean(BackgroundImagesPreferences.PREF_SHOW_BRAVE_STATS, true); | ||
| // return ChromeSharedPreferences.getInstance() | ||
| // .readBoolean(BackgroundImagesPreferences.PREF_SHOW_BRAVE_STATS, true); | ||
| return false; |
There was a problem hiding this comment.
we are already returning false here so changes in other files where this is being used is not needed
| @@ -23,8 +23,9 @@ public static void setDisplayTopSites(boolean shouldDisplayTopSites) { | |||
| } | |||
|
|
|||
| public static boolean shouldDisplayBraveStats() { | |||
| return ChromeSharedPreferences.getInstance() | |||
| .readBoolean(BackgroundImagesPreferences.PREF_SHOW_BRAVE_STATS, true); | |||
There was a problem hiding this comment.
changing true to false here would also set this preference as false and thus would return false? and we wouldn't need to comment the whole line. minimizing changes done in existing brave code is important because it will help speed up syncing with latest brave branch in future
There was a problem hiding this comment.
here func will first check what value does BackgroundImagesPreferences.PREF_SHOW_BRAVE_STATS returns if it returns true then func will return true, if false then func will return false and if the value does not exists then it will return true by default. So a possibility of returning true will still remain even if default value is set to false.
There was a problem hiding this comment.
If this default preference is not set anywhere else then it is being set from here. If we make this false by default then it'll return false the first time and since we have hidden all other options to change this preference therefore preference will never change to true
| @@ -43,7 +43,7 @@ | |||
| app:layout_constraintGuide_end="32dp" /> | |||
|
|
|||
| <!-- Keep the ID below as it's been tested in Chromium code by IncognitoNewTabPageStation. --> | |||
| <ImageView | |||
| <!-- <ImageView | |||
There was a problem hiding this comment.
what if we set a property here hiding these views. we won't have to comment the whole block of code then
There was a problem hiding this comment.
i will have to set the visibility none, ht, wd , padding, margin = 0. to keep the ui aligned
No description provided.